Skip to content

boot: bootutil: add support for multiple hw keys#2654

Open
jameswalmsley wants to merge 1 commit intomcu-tools:mainfrom
jameswalmsley:jw/multipl-hw-key
Open

boot: bootutil: add support for multiple hw keys#2654
jameswalmsley wants to merge 1 commit intomcu-tools:mainfrom
jameswalmsley:jw/multipl-hw-key

Conversation

@jameswalmsley
Copy link

  • Added support for multiple external keys when using the MCUBOOT_HW_KEY configuration.
  • Kept support for the original MCUBOOT_HW_KEY behaviour.

I'd appreciate any feedback so we can get this into a shape that can be merged.

:)

Copy link
Member

@d3zd3z d3zd3z left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, there's a tradeoff. Normally, I wouldn't want to add an ifdef to have multiple variants of the same function, but just change the function itself, and make the normal flow without the feature just have a loop of one iteration.

This is, however, a sort of public API.

However, I kind of feel that this is a change worth making, as the fix for implementers will 1. be flagged as a compilation failure, and 2. is easy to fix, accept the argument, and return BOOTUTIL_HW_KEY_NO_MORE if the index is greater than 0. I personal feel the API change is worth avoiding having to add yet more ifdefs to the code.

@jameswalmsley
Copy link
Author

So, there's a tradeoff. Normally, I wouldn't want to add an ifdef to have multiple variants of the same function, but just change the function itself, and make the normal flow without the feature just have a loop of one iteration.

This is, however, a sort of public API.

However, I kind of feel that this is a change worth making, as the fix for implementers will 1. be flagged as a compilation failure, and 2. is easy to fix, accept the argument, and return BOOTUTIL_HW_KEY_NO_MORE if the index is greater than 0. I personal feel the API change is worth avoiding having to add yet more ifdefs to the code.

Hi @d3zd3z - yes that was exactly my thinking - that I wanted to preserve the API - and not break anything for current users who have implemented this.

I'm happy with your suggested approach, and I'll update the PR so there's only a single API going forward.

Thanks for the feedback :)

Signed-off-by: James Walmsley <james@fullfat-fs.co.uk>
@jameswalmsley
Copy link
Author

@d3zd3z I've updated the patch - and I'm testing it on my boards.. I'll confirm that its working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants